Codecov Report

Merging #257 (ee59667) into master (f3fad96) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #257      +/-   ##
==========================================
+ Coverage   95.41%   95.43%   +0.02%     
==========================================
  Files           5        5              
  Lines         436      438       +2     
==========================================
+ Hits          416      418       +2     
  Misses         20       20              

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f3fad96...ee59667. Read the comment docs.

It really would be nice not to have to specialize so many things on OffsetArray. The more methods we have, the less generic we are. #255 (comment) seems like it needs a decision before we do things like this.

This is unrelated to bounds-checking though, and may be addressed independent of #255. This seems to be a necessary addition at least for ranges, and may be version-limited if Base decides to use traits to check if the argument is range-like in iterate. I can restrict this to OffsetRanges to avoid making the method too broad, if this is a concern.

My point is that it's better to address these by private methods or traits; we don't want to overload every exported function in Base specifically for OffsetArrays, that would be a nightmare. As much as possible, we should give them the generic interface and then let the generic implementations handle them.

I recognize that may not always be possible, but that should be the goal.

Another approach: what is the specific source of the poor performance on master? Can we fix it there instead?

That might be possible using a trait-based approach similar to #255. Perhaps the source of this difference, at least for OffsetRanges, is that iterate doesn't realize that the argument is range-like and supports rapid iteration using first, step and last without a need to index into the argument. The difference for arrays is a bit harder to track down, perhaps more efficient bounds-checking?